-
Notifications
You must be signed in to change notification settings - Fork 498
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Typealias RequestError to avoid KituraContracts import #1267
Conversation
Codecov Report
@@ Coverage Diff @@
## master #1267 +/- ##
=======================================
Coverage 88.15% 88.15%
=======================================
Files 37 37
Lines 2355 2355
=======================================
Hits 2076 2076
Misses 279 279
Continue to review full report at Codecov.
|
#1217 included a test, is that possible here? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks fine in principle, would like a test.
@@ -19,6 +19,11 @@ import LoggerAPI | |||
import KituraNet | |||
import KituraContracts | |||
|
|||
/// Bridge [RequestError](https://ibm-swift.github.io/KituraContracts/Structs/RequestError.html) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need a dummy class like https://github.com/IBM-Swift/Kitura/blob/a8144547ed284befec973a0154790f208eb234d1/Sources/Kitura/HTTPStatusCode.swift#L20 ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As far as I can tell, this dummy class was only necessary because the file otherwise contains only a typealias
. In this case I dropped the typealias into the existing CodableRouter.swift
since I could argue it belongs there.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK as long as Jazzy is generating the typealias OK then we're fine :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tested locally, it generates fine.
And also enable the existing bridging test
Hmm, that's interesting, my test fails to compile on Swift 4.0.3. It appears I inadvertently used some Swift 4.1 feature. |
Description
In a similar vein to #1217, this adds a
typealias RequestError
to Kitura, to avoid a user having to explicitlyimport KituraContracts
when defining codable routes.Motivation and Context
While creating a minimal code example of use of codable routing, @helenmasters noted that we need to import KituraContracts because of the
RequestError?
in the declaration of signature for a codable handler. This allows us to skip that import.There are other situations where it may still be necessary to import KituraContracts, such as when defining a custom
Identifier
type, however the import can be confined to the source file that defines the type.How Has This Been Tested?
A simple example of codable routing now compiles without importing KituraContracts.
The Kitura tests passed locally.
Checklist: